feat: support reading service account tokens from CSI secrets field for Kubernetes 1.35+#2305
Conversation
|
@andyzhangx what's the best way to handle this in the helm charts? do you create a new version of chart for every Kubernetes release? for 1.35+, we should have |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
There was a problem hiding this comment.
Pull request overview
Adds Kubernetes 1.35+ compatibility for the CSI “serviceAccountTokenInSecrets” migration by preferring service account tokens from the CSI request Secrets field while keeping backward compatibility with VolumeContext.
Changes:
- Add
getServiceAccountTokens()helper to preferNode(Stage|Publish)VolumeRequest.SecretsoverVolumeContext. - Update NodePublishVolume/NodeStageVolume workload-identity gating logic to use the helper.
- Add unit tests covering helper precedence and nil-map behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/blob/nodeserver.go | Reads SA tokens from req.Secrets (preferred) with fallback to VolumeContext, and threads Secrets into an internal NodeStageVolume call. |
| pkg/blob/nodeserver_test.go | Adds unit tests for getServiceAccountTokens() precedence and nil handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Should we enable the feature in this PR by setting |
we should! |
@aramase, I discussed with @andyzhangx - you can add the |
…or Kubernetes 1.35+ Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
GetAuthEnv reads serviceAccountTokenField from attrib only, so the token delivered via Secrets (k8s 1.35+ serviceAccountTokenInSecrets) never reached the workload identity auth path. Copy attrib and inject the resolved token before calling GetAuthEnv in NodeStageVolume.
Gate on requiresRepublish in the helm template since the kube API server rejects serviceAccountTokenInSecrets when requiresRepublish is false.
7b92102 to
526616e
Compare
- Add feature.serviceAccountTokenInSecrets (default: false) in values.yaml - Gate serviceAccountTokenInSecrets in CSIDriver template on its own flag instead of piggy-backing on requiresRepublish - Enable by default in E2E_HELM_OPTIONS for pull-blob-csi-driver-e2e tests - pull-blob-csi-driver-e2e-proxy tests use default (disabled) since they don't set EXTRA_HELM_OPTIONS to override Signed-off-by: Andy Zhang <andyzhangx@live.com>
Signed-off-by: Andy Zhang <andyzhangx@live.com>
Chart.yaml used 'v0.0.0' which is not a valid semver for helm package. Fix to '0.0.0' and regenerate the tgz with the correct filename. Signed-off-by: Andy Zhang <andyzhangx@live.com>
Signed-off-by: Andy Zhang <andyzhangx@live.com>
The deploy/csi-blob-driver.yaml must remain compatible with older Kubernetes versions that don't recognize this field. Users on 1.35+ can enable it via the Helm chart feature flag instead. Signed-off-by: Andy Zhang <andyzhangx@live.com>
There was a problem hiding this comment.
/lgtm
I have made some changes to disable serviceAccountTokenInSecrets: true (enable in e2e test by default) in CSIdriver by default, so this version could still run on k8s cluster version < 1.35. We need to keep backward compatibility in the upstream version since many users are still running older k8s versions.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, aramase The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind feature
implementing changes for KEP: kubernetes/enhancements#5538
see https://github.com/kubernetes/enhancements/blob/master/keps/sig-storage/5538-csi-sa-tokens-secrets-field/README.md#driver-migration-example for why we're doing this.